Skip to content

Agent-loop hardening for small local models (Code mode) - #655

Open
timtoole02 wants to merge 33 commits into
codex/web-code-revivalfrom
codex/web-code-revival-agentwork
Open

Agent-loop hardening for small local models (Code mode)#655
timtoole02 wants to merge 33 commits into
codex/web-code-revivalfrom
codex/web-code-revival-agentwork

Conversation

@timtoole02

Copy link
Copy Markdown
Owner

Three commits of agent-loop hardening on top of codex/web-code-revival, sized for the
local-4B reality where every wasted round trip is a full decode pass.

What's here

fix(parse): the JSON escape repair desynced after an escaped quote, so a shell command
containing \" and \$ together stayed unparseable and the turn died on the malformed-syntax
guard. The escape pair is now consumed atomically; the live failing call is pinned as a test.

fix(checkpoint): undo now restores before forgetting (a failed restore no longer erases the
checkpoint and truncates the target), drops unrecoverable entries instead of wedging, sequences
undone_* parks, and rejects journal lines whose backup path resolves outside
.camelid/checkpoints — the journal is workspace-writable, so an absolute path smuggled into it
could read arbitrary files into /changes output.

feat(agent): small-model steering and recovery for the Code loop —

  • confined-path guidance in the system prompt + an actionable sandbox-escape error
  • one [hint: …] line on failed run_shell results for known failure classes (head+tail scan)
  • reciprocal tool-routing clauses (file tools name the shell commands they replace, and vice versa)
  • tool-name repair ladder (WriteFile/write-file/write_file_tool execute; ambiguity refuses;
    echoed/garbage names get a terse catalog-free error)
  • output-cap truncation classified before the malformed-syntax guard
  • oversized tool batches clamp and defer instead of killing the turn
  • run_shell honors Stop inside its wait loop (~50ms instead of the full shell timeout)
  • successful shell work satisfies the completion contract via a bounded fail-closed mtime scan
    (1s slack for HFS+ volumes, VCS/build dirs skipped)
  • budget exhaustion asks for one toolless final summary; terminal outcome stays honest
  • reasoning-only replies resume from their own reasoning instead of being re-asked

Verification

  • cargo test --all-targets: 2406 passed / 0 failed (66 binaries), macOS arm64
  • cargo clippy --all-targets -- -D warnings: clean (also fixes two pre-existing failures of
    this gate on the non-Windows legs)
  • Every behavior change carries a unit test; live receipts on an M4: a sandbox-denied command
    was not retried, a bulk-create request completed as one shell loop with verified files, and a
    truncated write_file received the cap correction with zero malformed strikes.

🤖 Generated with Claude Code

The in-string tracker flipped on every raw '"', so an escaped quote inside a
tool-call argument desynced it and the later invalid escapes (a shell \$ in a
run_shell command) were never doubled. The call stayed unparseable, the model
re-emitted it verbatim, and the turn died on the malformed-syntax guard.
Consume the escape pair atomically instead, and pin the live failing call as a
regression test.
- Restore BEFORE forgetting: a failed restore used to erase the checkpoint from
  both stores and leave the target truncated. Now a retryable failure re-pushes
  the entry (idempotently), while an unrecoverable one (missing/out-of-store
  backup blob) drops it with an explanatory error instead of wedging undo.
- Re-validate the backup path at use time and reject journal lines whose backup
  resolves outside .camelid/checkpoints: the journal is workspace-writable, and
  an absolute path smuggled into it read arbitrary files into /changes output.
- Sequence undone_* parks like prepare() sequences backups, so a second undo of
  the same file no longer overwrites the only copy of the intermediate state.
Tests cover the failed-restore retry, the escaping-backup rejection, and park
collisions.
Steering and recovery, sized for a local 4B where every wasted round trip is a
full decode pass:

- Confined sessions now state the path rule (workspace-relative, never absolute)
  in the system prompt; the sandbox-escape error names the correction instead of
  advertising the CLI-only --allow-fs flag.
- Failed run_shell results append one actionable [hint: ...] line for known
  failure classes (sandbox denial, missing interpreter, compile/test verdicts,
  full disk, denied network), scanning both the head and tail of the output.
- Tool descriptions route both directions: file tools name the shell commands
  they replace, run_shell defers to them and to one-loop bulk work.
- Near-miss tool names (WriteFile, write-file, write_file_tool) are normalized
  and fuzzy-repaired to the advertised name instead of burning a validation
  strike; echoed/garbage names get a terse catalog-free error so phantom calls
  are not primed. Repairs refuse ambiguity and never swap one real tool for
  another.
- A step cut at the output cap is classified as capped BEFORE the malformed-
  syntax guard, so a truncated write_file gets the split-the-work correction
  rather than a malformed strike.
- Oversized tool batches clamp to the per-step limit and defer the remainder
  with an explicit note, instead of killing the turn.
- run_shell honors the turn's cancel flag inside its wait loop: Stop lands
  within one poll instead of being ignored for the whole shell timeout.
- A successful shell command that actually changed the tree satisfies the
  completion contract: a bounded fail-closed mtime scan (1s slack for HFS+
  volumes, VCS/build dirs skipped) reports the real changed paths into the
  semantic post-change capture.
- Budget exhaustion asks for one toolless final summary so a spent turn leaves
  a partial deliverable; the terminal outcome stays StepCapped/Repeated.
- A reasoning-only reply resumes from its own reasoning (carried inside the
  correction message) instead of being re-asked or committed as the answer.
- cfg_attr allows for the two Windows-only paths that failed clippy -D warnings
  on the macOS/Linux legs.

Each behavior is pinned by a unit test; the live receipts are a sandbox-denial
turn that stopped retrying and a bulk-create turn that finished in one shell
loop with verified files.
The scrub guard rejects any /Users/<name> literal; a hint test used one as a
sample denied path, so swap it for /etc/probe (the arm matches on the error
phrase, not the path). Also apply rustfmt to the new conditionals.
Killing the direct child does not necessarily close the pipe write ends on
Unix: a pipe reports EOF only once EVERY writer has closed, and `/bin/sh -c`
may leave a descendant holding the inherited fd. Joining the reader threads
after the kill therefore waited for that orphan to exit on its own, so both the
deadline and a user Stop were silently unbounded — Linux CI measured a full 30s
for a cancel that was requested before the first poll.

Detach the readers on both abnormal-exit paths instead; their output is
discarded there, and they own only their own buffer. The regression test now
uses a backgrounded descendant that inherits the pipes, which is the shape that
reproduced on Linux while passing on macOS (where `sh -c` execs a lone command
and the fds die with it).
Both predate this PR — the branch's own CI run at e169eda failed the same two
jobs — but they block every PR that targets it, so they are fixed here rather
than left to fail alongside work that is green.

validation-scripts: the SmolLM3 qualification pins record the git blob SHA of
src/api/mod.rs as provenance for the renderer. This branch edited that file for
the web-code routes, so the blob moved and four derived pins went stale (the
runtime-envelope fixture, the parity script's RENDERER_GIT_BLOB_SHA1 and its
copy of the fixture digest, and the test's double-entry tripwires for both).
Regenerated all four. The thing the pins protect is unchanged: the renderer
fn is byte-identical to main (45 lines, sha1 7aecfc97) and the branch's diff to
src/api/mod.rs contains no smollm3 lines, so this is stale provenance, not a
behavior change.

rust (windows): windows_store_alias_failure_requires_py_launcher_probe drives a
REAL `python --version` and asserts it hits the Store alias stub, so it can only
pass on a host where that is what python resolves to. GitHub's windows runner
ships a working Python, so the assertions could never hold there and the job
failed on every run regardless of the code under test. It now self-skips when
the host does not reproduce the alias, the same way the Metal tests skip with no
device. Verified on a Windows box where the alias IS in play: the test passes
there, and `where python` shows the WindowsApps stub shadowing a real Python
3.10 — so the guard keys on the observed behavior of `python --version`, not on
whether Python is installed anywhere.
The auto `py -m py_compile` probe borrows the caller's shell timeout, so on a
loaded machine it can fail for reasons that say nothing about the file: a
timeout, a spawn failure, a missing launcher. Every such failure was recorded as
"Python syntax validation failed", which lies to the model about its own
correct source AND — because semantic_contract_findings is sticky — re-arms the
completion gate on a finding that can never be re-derived, so the turn ends
Repeated instead of Answered.

That is the flake on the Windows CI leg: the same commit (811def4) failed three
tests on one run and passed 1907/0 on another, and the three failures are
exactly this shape (two Repeated-instead-of-Answered, one missing SyntaxError
that the probe never got to produce).

Classify before blaming the file, mirroring the discrimination the alias and
traceback checks in this same file already apply. A host failure is now
disclosed as unverified rather than reported as a defect. The rule lives in a
platform-independent helper so it is testable on every host, though its only
caller is Windows-only.
…ermes

Every item targets the same metric: work per model round trip, because each
step on the local 4B lane is a 10-20s decode.

- Shell output clips head+tail instead of head-only, after stripping ANSI.
  Build and test runners put the verdict at the END, so a chatty cargo run
  handed back 16 KiB of banner and dropped the failing assertion entirely; the
  only recovery was re-running the whole command piped through `tail`. The
  tail-inclusive splitter already existed in this file but was reachable only
  with context paging on.
- edit_file gains a match ladder (CRLF fold, whitespace-trimmed lines, Unicode
  confusables) and, when it still misses, returns the file's ACTUAL text for
  the region instead of a bare "not found". Exact-match-only turned cosmetic
  drift into two failed calls and then a whole-file rewrite — the most
  expensive path in the loop.
- edit_file gains replace_all, and the ambiguity error now names it.
- edit_file echoes the changed region back, so the model stops spending a
  round trip re-reading to confirm its own write.
- read_file numbers both branches identically with an ` | ` anchor that cannot
  occur in source, and both tools state that the prefix is not file content.
- The per-step tool-call budget is advertised; the two recovery messages that
  said "exactly one tool call" are scoped to the output-cap branch they belong
  to rather than reading as the standing contract.
- The repeat/churn signature is keyed on the REPAIRED tool name, so varying the
  casing no longer evades the guard, and exact duplicates inside one batch are
  collapsed before execution.
- Path-not-found errors suggest similar siblings (substring or edit distance)
  and state that paths are workspace-relative.
- Mid-turn corrections are appended as tagged <system-reminder> user turns
  instead of AgentMsg::System. System messages were folded into the FIRST user
  message, so a step-12 correction was injected retroactively at position 0 AND
  rewrote the prompt prefix, discarding the prefix cache on every correction.
  Reminders are excluded from every "last user request" lookup — without that
  a correction silently became the request.

Tool schemas now have their own token-budget guard, so future growth is a
deliberate decision rather than an unrelated capsule failure.
…trips

Mined from earendil-works/pi. The headline is a prefix-cache bug, not a
missing feature.

compile_history_for_step rebuilt a GROWING evidence blob on every workspace
step and placed it immediately after the user's goal, so the token sequence
diverged at the FRONT of the turn and every step re-prefilled the whole thing.
Emit one Memory message per observation instead: earlier observations stay
byte-identical between steps, so the shared prefix now runs through all of them
and only the newest tool group differs. Prefill dominates wall clock on this
lane, so this was the largest avoidable cost in the loop.

The prefix cache also refused every constrained request outright. The hazard
that motivated the refusal (recorded as B1) was real but narrow, and is now
handled at the point of use rather than avoided: the exact-hit path masks the
cached logits with the constraint's first-token set AND commits the sampled
token to the grammar state, so the next mask is computed from a correctly
advanced state; the partial-hit path already re-enters the masked main loop.
Storing is constraint-independent — the artifact is prompt KV plus raw logits.
This is the prerequisite for grammar-constrained tool calls.

Also:
- The compaction digest records tool-call PATHS, not just names, so a compacted
  model stops re-reading files it already read. The path comes from the agent's
  own arguments, so this does not retain tool output.
- A transient model-step failure is retried (bounded, backed off) instead of
  discarding a turn that already paid for every prior step. The retry prompt is
  byte-identical, so it rides the prefix cache.
- search reports WHY it stopped — hit cap, file budget, or time budget — instead
  of one catch-all whose advice ('narrow pattern or path') was actively wrong
  for the hit cap, since limit is a parameter the model can raise. Per-hit lines
  are capped so one minified line cannot evict every other match.
- Truncation notices are anchored: how much was shown, the total, and the exact
  read_file continuation. An unanchored marker left a small model with only two
  moves, both dead decodes.
- parse_args gains a schema-directed coercion rung ('50' -> 50, 'true' -> true,
  dropping explicit nulls on optional fields) before the hard reject, and
  edit_file's replace_all accepts the same lenient spellings.
- Tool descriptions are no longer sent twice per request: the system prompt
  lists name and risk, the JSON schema carries the description.
A Q8_0 model runs a resident F32 KV primary and was locked out of the
prompt-prefix cache entirely, so every step of a multi-step turn
re-prefilled the whole prompt. The lockout was real but the cause was one
line: the shared GPU->host mirror routed through `store_kv_head_row`,
which rounds every value through f16, so `kv_roundtrips_through_cpu_exactly`
answered false and both lookup and store were refused.

That rounding is a deliberate llama.cpp-oracle contract
(`kv_cache_storage_matches_llama_cpp_f16_rounding`), but it is load-bearing
only for K/V the CPU forward computed. Mirrored K/V never touched the CPU
reference lane, and its own doc justified the rounding as idempotent
"because the GPU KV is f16 already" — false for an F32 primary, and that
false assumption was the whole defect.

So thread a `KvStoreFidelity` from each engine's own primary format into
the mirror. Metal's F32 primary writes `ExactF32`; F16/Q8 and CUDA keep
`F16Rounded` (CUDA converts back to f16 bits when it re-seeds, so an exact
copy would be discarded). Both halves of the Metal round trip are already
plain copies, so with the rounding gone the trip is bit-exact, and the
cache gate is derived from the same engine state as the mirror so the two
cannot drift.

Measured, Qwen3-4B-Q8_0, four steps of a growing ~2.6k-token prompt:
73.2s and 0 cache hits -> 52.6s and 3 hits, decode 22.1 -> 21.8 tok/s.

Explicitly NOT done: moving Q8_0 onto the F16 primary to make the old gate
pass. That reaches the same wall-clock but silently disables the split-K
decode attention and the attention-as-matmul prefill (both gated on
`!kv16`) and changes the attention numerics — 16 Metal-vs-CPU parity tests
fail. A comment on `resident_kv_format` now records that.

Also closes a latent bug: a session that took one CPU fallback was
advertised as cacheable by the `cpu_kv_authoritative` early-accept, and its
f16-rounded rows were seeded back into the F32 GPU cache. Fixing fidelity
inside the mirror covers that path too.
Every tool call rendered as a full-width bordered card: status disc, bold
name, a "Tool completed" subtitle, and a right-aligned Done pill. Four
pieces of chrome for one line of information, repeated per call, with a
full-width `11.5s · 32 tokens · 11540ms` strip between each pair. Five
calls read as five boxes instead of five words.

A tool call is now one quiet line — glyph, name, what it acted on, chevron
— that expands in place. Failures state what went wrong on the line
itself, so they need no card either. The only element that keeps its box
is a pending approval, which blocks the run and carries four consequential
actions; containment is the point there.

Model-step metrics move inside the expansion. They are reported the
instant a step returns, BEFORE the tool call that step produced, so they
are the cost of the work that follows them — painting them beside the tool
name attributed the seconds to the wrong subject.

Also fixes prose that was never styled at all: every rule in chat.css is
scoped to `.cxturn__body`, which this view does not enter, so assistant
paragraphs had zero spacing and headings rendered at body size.

The inspector becomes six dense groups over data that already crossed the
wire unrendered. `memory.updated` carries ten numeric fields and had no
render branch at all — it is now a context-budget meter and a composition
breakdown. `model.timing` becomes a per-step table. The plan parse moves
to the reducer so the transcript card and the sidebar read one parse.

Totals accumulate in the reducer rather than being scanned off
`state.events`: that array is a 240-entry ring, so a scan would make a
long run's totals drift DOWNWARD. Same reasoning as `liveTurns`.

Deliberately NOT built: an Agent | Model | Tokens | Time table. A subagent
runs in a separate process whose reporter no-ops everything but
model_text/tool_call, so a child emits zero events into the parent stream;
SubagentResult carries no token count; and children inherit the parent's
model_id, so that column would print one string on every row. The fields
needed are recorded for a backend change instead of faked.

Declares `--color-border`, referenced by five rules and defined nowhere —
the composer had no border and no focus ring, and `color-mix()` against an
undefined token silently voided the rule.

Adds this view to the smoke's brand-name guard; it was outside the list
and check-public-scrub.sh has no such pattern, so nothing checked it.

Gates: ui, contrast, code-workbench, workspace, workspace-visual,
markdown all pass; bundle 127.9 kB gz against a 229.9 kB budget.
`write_file rpncalc/__init__.py` into a fresh workspace failed with
"cannot access parent of rpncalc/__init__.py: No such file or directory",
and so did every other write into a directory that did not exist yet.
Laying down a package — the ordinary shape of a from-scratch task — cost
one failed call per file before the model could make any progress, and the
only route out was for it to guess `run_shell mkdir -p`, which is a
separate approval on the gated surfaces. There is no mkdir tool.

Two causes, both fixed here:

`Sandbox::resolve(.., must_exist=false)` canonicalized the parent
directory, which cannot succeed when the parent is what is missing. It now
canonicalizes the nearest EXISTING ancestor and re-appends the missing
components. Canonicalizing is what resolves `..` and symlinks, and it is
the only reason the `starts_with(root)` check means anything — so the
reconstructed tail must not contain `..`, which cannot be resolved against
a real directory and would otherwise walk back out of the workspace past
the check. That case, and anything else unresolvable, fails closed.

`write_file` then creates the parent. The path has already been through
`resolve`, so this cannot create a directory the write itself would not
have been allowed to target.

Both halves are pinned: one test writes a package (including a three-deep
path) and reads it back, the other asserts three `..` escapes through a
missing parent are still refused while a legitimate deep path is allowed.
Chats have delete; coding sessions did not, so code history only ever grew.
The backend already had the endpoint — DELETE
/api/agent/workspace/threads/:id, complete with a guard that refuses to
drop the thread of a still-running session — so this is the missing
frontend half.

The row becomes a container holding an open control and a delete control.
It was a single <button>, and a button inside a button is invalid and
unreachable by keyboard, so the row could not simply gain a second one.
The delete control is revealed on hover and on keyboard focus, and toggles
`visibility` rather than `opacity` alone so it does not sit invisible in
the hit-test layer swallowing clicks meant for the row.

The endpoint requires a `workspace` query parameter and cross-checks it
against the thread's stored canonical_root; omitting it fails the request
before the handler runs ("missing field `workspace`"), so the thread's own
root is passed. The confirm dialog names the workspace and says plainly
that files the session wrote stay on disk, and a refused delete shows the
server's own message instead of closing on a delete that did not happen.

Gates: ui, contrast, code-workbench, workspace all pass.
A goal naming the module layout, the constraints, the CLI surface and the
acceptance criteria — the shape that actually makes an agent succeed — was
rejected outright with "goal must contain 1 to 4096 UTF-8 bytes", after
the author had written the whole thing.

4 KiB was an abuse bound doing a job it is bad at. What a goal can afford
is the model's context window, and the context budget plus auto-compaction
already enforce that downstream with real numbers. Raise the hard cap to
64 KiB and let that machinery size the prompt.

The rejection also only quoted the rule, which leaves the author guessing
how far over they are. It now reports the actual size and how much to trim.
The same limit and message cover follow-up messages, which had the same cap.

The inspector rendered the whole task text in the agent row, so a spec of
that size filled the panel end to end and pushed Changes, the context meter
and every other group off the bottom. Clamped to three lines, with the full
text on hover; it is also the transcript's first message.

Full suite 2421 pass, clippy/fmt/scrub clean; ui, contrast and
code-workbench gates pass.
The SSE response body owned the turn. It held a CancelStreamOnDrop guard
whose Drop called WorkspaceBridgeControl::cancel, and it owned the only
event receiver, so the forwarder's next send failed and cancelled again. A
refresh, a tab close, a sleeping laptop and a dropped packet are the same
event at that layer, and all four ended a run that takes 5-20 minutes here.

That guard could not simply be deleted, because it was also the ONLY bound
on a Code turn: workspace_max_steps returns 0 for Code (no step cap), and
Code alone got set_stream_cancel with no wall-clock deadline. Removing it
naively would have traded a lost-run bug for an unbounded-run bug.

So the turn is decoupled from any socket, and the bound moves to the
server. A turn now ends when it finishes, when the user Stops it, when the
loop trips its own guard, or when a supervisor decides nobody wants it:

* The worker and forwarder start at turn-install time, not when a browser
  opens /events. There is no claim to lose, so WaitingForEvents is gone.
* A retained session-scoped feed replaces the per-stream channel, so N
  readers and zero readers are both legal, and a reconnecting reader
  resumes from `?after=` or Last-Event-ID.
* A per-turn supervisor owns orphan protection with three monotonic
  deadlines: 30s if no browser ever attached, 90s once one attaches and
  leaves, 2h unconditional.

Liveness is a delivery watermark, not a refcount. A half-open TCP peer
holds observers at one forever, so "watching" means the feed is not ahead
of what a reader actually wrote out. That also restores the bound the
removed try_send-on-Full kill used to provide.

Worst case a turn runs ~215s unwatched: 90s grace + 5s tick + one in-flight
tool (run_shell is capped at 120s) + a =250ms cancel check. Every reaper
predicate fails CLOSED on a poisoned lock, because a reaper whose inputs an
unrelated panic can disable is a reaper that stops reaping.

Client side: a lost stream reconnects instead of giving up, adoption
re-attaches rather than merely watching, the in-flight turn's own prompt is
re-seeded from the activity snapshot so the transcript is not an answer
with no question, and readRecordedOutcome returns null rather than
stamping an answered turn "Stopped".

Dedupe resets its cursor for a new turn and keeps it only across a
re-attach. The server numbers monotonically per session and would not
restart, but a client that works only while the server keeps that
discipline breaks silently the day it changes — and silently here means an
empty transcript for a run that is really executing.

The workbench fixture emitted sequences out of order within one stream
(4,3,3,3,4), which modelled no server and which dedupe correctly dropped —
taking the update_plan card with it. Renumbered monotonically, and its
abort terminal renumbered above the 225-event ring flood for the same
reason. The follow-up stream still restarts at 1 on purpose: that is the
new-turn cursor reset under test.

Known landmine: the reaper relies on the process dying abruptly. axum::serve
has no with_graceful_shutdown today; if that ever lands, a Drop backstop on
the supervisor future must land with it or orphans become real.

Rust 2426 pass, clippy/fmt/scrub clean. Frontend: workspace, ui, contrast,
code-workbench, workspace-visual, markdown, streaming all pass.
timtoole02 added a commit that referenced this pull request Aug 14, 2026
"this step's reply is limited to N tokens by the remaining context budget"
was a reporter notice, which renders as prose between the agent's actions —
the one place a reader is following what it DID, not how it was budgeted.

The same fact already goes out as structured data on the very next line,
where the UI renders it as an always-visible context meter that never
interrupts the flow.

The `trimmed` notice above it stays: that one reports something LOST, which
the reader cannot see any other way.
Nine observed TaskForge runs on Qwen3-4B produced the same two failures,
and the difference between the ones the model recovered from and the ones
it looped on was entirely whether the message named a next action.

An unterminated f-string — a literal newline inside `f'…'` — appeared in
ALL NINE runs, always in `add_task`, and survived correction: in one run
the model fixed line 51, later regenerated the file, and re-emitted the
identical break. The kernel has said "spell line breaks as \n, never as raw
newlines" since 5f0705e and is ignored, so instruction is not the lever.

`write_file` now lints it and refuses BEFORE the file lands, quoting the
offending line and the fix. Catching it at authorship costs one rejection;
catching it downstream costs a write, an execute, a syntax failure, a read
and an edit — and leaves a file that does not parse if the turn ends first.

The lint is deliberately narrow: single-quoted f-strings only, triple
quotes skipped (legally multi-line), non-f-prefixed strings ignored,
escapes honoured. False negatives are fine because the execution syntax
check still sits behind it; false positives are not, because refusing a
legitimate write strands the agent with no way to author the file.

`list_dir` on a file returned `Not a directory (os error 20)`. That names a
POSIX condition, not a next step, and the model retried it three times in
one run — the same run where it recovered first-try from every failure
whose message named the fix. It now says the path is a file and routes to
`read_file`.

One existing test moved off an unterminated f-string as its fixture. Its
subject is that a syntax-broken file which REACHED DISK keeps the Verify
gate execution-only; the lint would have intercepted the write and tested
itself instead. It now uses a defect the lint deliberately does not catch,
which is the realistic case for the backstop.

2547 pass, clippy/fmt/scrub clean.
wait_until_completed() returns for Error as well as Completed, and an errored
command buffer has run none of its dispatches. The qwen35 lane was the only
resident lane in this file that never inspected status() afterwards.

forward_prefill_batch therefore advanced `filled` and reported success even when
nothing executed, leaving cache_k/cache_v/conv_state/state at the values reset()
left while the caller believed the prompt was resident. Decode then generated
from an empty KV cache and a zero recurrent state, emitting fluent in-vocabulary
tokens unrelated to the prompt, at temperature 0, with no error raised anywhere.
That prefill chunk is the largest submission the process makes, so it is the one
a watchdog kill or a working-set allocation failure claims, and the command queue
is process-global, so one faulted buffer takes the whole run rather than a turn.

forward_select had the same hole with a worse payload: the greedy token id came
straight out of a recycled scratch buffer the pool never zeroes. pool_get rounds
to a power-of-two size class, so every small scalar in the engine shares that
bucket; those values land inside the vocabulary and decode to real tokens.

Gate all four sites on MTLCommandBufferStatus::Completed and fail closed so the
caller falls back to the CPU lane instead of inventing output. Each site logs:
nothing recorded status() before, and that silence is why the symptom read as a
model-quality problem for as long as it did.

CAMELID_QWEN35_FAULT_INJECT is default-off and forces the condition, so the
recovery path is testable. Injected, the guard fires, falls back, and the answer
is still correct; on the healthy path it never fires.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant